Advance filtered AllStreamSubscription checkpoints on server checkpointReached#554
Advance filtered AllStreamSubscription checkpoints on server checkpointReached#554alexeyzimarev wants to merge 5 commits into
Conversation
…ents Wire up the checkpointReached callback on SubscriptionFilterOptions so server-reported checkpoint positions flow through the same ordered commit machinery as real events, as payload-less contexts (Message is null, so Handler ignores and acks without touching the consume pipe). Without this, a filtered $all subscription's checkpoint only advanced when a filter-matched event was processed. A long unmatched stretch (sparse filters, quiet servers) left the checkpoint parked at the last matched event: restarts re-scanned everything since then, and any consumer comparing the checkpoint to the $all head saw a phantom, never-closing lag. Adds CheckpointReachedTests, which reproduces the bug with a filter that matches nothing and asserts the checkpoint still advances past written events.
PR Summary by QodoAdvance filtered $all checkpoints using server checkpointReached callbacks
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
Code Review by Qodo
1.
|
…-less contexts Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds CheckpointGatedByPendingMatchTests, which reproduces the safety half of the checkpointReached fix: a filter-matched event whose ack is held open must block the stored checkpoint from advancing past it, even while checkpoint markers for unmatched noise keep arriving. The commit handler's "Commit" diagnostic (fired before the sequence gate) proves a marker positioned BEYOND the blocked event reached the commit path during the hold — not assumed from a fixed sleep, and not satisfiable by markers for pre-existing activity below the event — and the blocked handler is always released so a failed assertion can't hang fixture disposal. Adds CheckpointCommitHandlerBackpressureTests, which pins the channel backpressure CheckpointCommitHandler switched to instead of throwing when the commit queue overflows, and that disposing with a writer parked on the full channel releases it (ChannelClosedException) and drains the queue without hanging, force-recommitting the last stored position. The store gate always opens and the handler is always disposed on failure paths, so a caught regression surfaces as an assertion failure rather than a timeout.
…sticCounter Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Test Results 105 files + 83 105 suites +83 41m 41s ⏱️ + 28m 40s For more details on these failures, see this check. Results for commit 88b263d. ± Comparison against base commit c55af3b. This pull request removes 5 and adds 29 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Problem
AllStreamSubscription.SubscribeconstructsSubscriptionFilterOptions(filter, checkpointInterval)without the thirdcheckpointReachedparameter, so the callback defaults to a no-op. A filtered$allsubscription's checkpoint therefore only advances when a filter-matched event is processed:$allhead (including theeventuous.subscription.gap.countmetric) sees a phantom, never-closing lag.Fix
Wire
checkpointReachedto route the server-reported checkpoint position through the existing ordered commit machinery as a checkpoint-only message: a null-payloadMessageConsumeContext(type$checkpoint-reached) with the position and the subscription'sSequence++, handed toHandleInternal. The null-message arm inEventSubscription.Handleralready Ignores + Acknowledges such contexts, which lands inCheckpointCommitHandler.Commitwith an ordered sequence number — no new commit paths.Why the ordering is safe
eventAppearedandcheckpointReachedsequentially on one delivery loop (each awaited beforeMoveNextAsync), soSequence++never races event contexts and sequences reflect exact delivery order.CheckpointCommitHandler's contiguous-sequence gate (CommitPositionSequence.FirstBeforeGap) holds a gap position back until every lower-sequence event has acked. The checkpoint can never commit past an unprocessed event, so crash/restart semantics are unchanged.Sequence.Test
CheckpointReachedTests.CheckpointAdvancesPastUnmatchedEvents: a filtered subscription whose filter matches nothing, 500 unmatched events appended — asserts the handler saw zero events AND the stored checkpoint advances past the last event's position (bounded 30s poll + 60s timeout). Verified red without the fix, green with it.Notes
CreateContext.SubscriptionActivityfor async contexts when diagnostics are enabled; gap contexts now hit that arm periodically. Harmless inert allocation — happy to short-circuit it here or in a follow-up if preferred.🤖 Generated with Claude Code